Skip to content
This repository was archived by the owner on Apr 22, 2024. It is now read-only.

chore: updating eth-json-rpc-middleware#398

Closed
martinheidegger wants to merge 1 commit into
MetaMask:mainfrom
tradle:update/eth-json-rpc-middleware
Closed

chore: updating eth-json-rpc-middleware#398
martinheidegger wants to merge 1 commit into
MetaMask:mainfrom
tradle:update/eth-json-rpc-middleware

Conversation

@martinheidegger
Copy link
Copy Markdown

In order to update the eth-json-rpc-middleware the requirestatements had to change, but other changes in v8 are minor and the changes in v7 do not seem to impact any use.

@martinheidegger martinheidegger requested a review from a team as a code owner January 17, 2022 15:16
Copy link
Copy Markdown

@httpJunkie httpJunkie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, upgrade package and change imports due to having a named import vs default import prior.
In this review I would note that I do not know if there is any issue with running this specific version and what impact that may have, but the changes are minimal and expected considering a basic update to a new version.

Copy link
Copy Markdown
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @martinheidegger — sorry for the delay in responding to this. Changes mostly look good, just a few suggestions based on how eth-json-rpc-middleware should be used now.

Comment thread subproviders/cache.js
@@ -1,5 +1,5 @@
const ProviderSubprovider = require('./json-rpc-engine-middleware')
const createBlockCacheMiddleware = require('eth-json-rpc-middleware/block-cache')
const { createBlockCacheMiddleware } = require('eth-json-rpc-middleware/dist/block-cache')
Copy link
Copy Markdown
Contributor

@mcmire mcmire Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing exports beyond the main file are no longer recommended (and in the future may be prohibited). You should be able to say:

Suggested change
const { createBlockCacheMiddleware } = require('eth-json-rpc-middleware/dist/block-cache')
const { createBlockCacheMiddleware } = require('eth-json-rpc-middleware')

Comment thread subproviders/fetch.js
@@ -1,5 +1,5 @@
const ProviderSubprovider = require('./json-rpc-engine-middleware')
const createFetchMiddleware = require('eth-json-rpc-middleware/fetch')
const { createFetchMiddleware } = require('eth-json-rpc-middleware/dist/fetch')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { createFetchMiddleware } = require('eth-json-rpc-middleware/dist/fetch')
const { createFetchMiddleware } = require('eth-json-rpc-middleware')

@@ -1,5 +1,5 @@
const ProviderSubprovider = require('./json-rpc-engine-middleware')
const createInflightCacheMiddleware = require('eth-json-rpc-middleware/inflight-cache')
const { createInflightCacheMiddleware } = require('eth-json-rpc-middleware/dist/inflight-cache')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { createInflightCacheMiddleware } = require('eth-json-rpc-middleware/dist/inflight-cache')
const { createInflightCacheMiddleware } = require('eth-json-rpc-middleware')

@mcmire
Copy link
Copy Markdown
Contributor

mcmire commented Jan 4, 2023

Revisiting this pull request. Thank you for the submission, but we are actively phasing out this package in favor of json-rpc-engine and its related packages, so I'm going to close this. As for the changes here, we recommend you use eth-json-rpc-middleware directly instead of going through this package.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants